Skip to content

Tighten the rx.client_state contract before 0.11.0 - #7804

Merged
FarhanAliRaza merged 3 commits into
mainfrom
fix/client-state-api-contract
Nov 11, 2026
Merged

FarhanAliRaza merged 3 commits into
mainfrom
fix/client-state-api-contract

Conversation

@masenf

@masenf masenf commented Nov 11, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes FINDING-034 and FINDING-035 from the reflex 0.11.0a1 pre-release testing (artifacts on branch claude/vibrant-pasteur-3921g3, prerelease-testing/runs/2026-11-11-v0.11.0a1/client_vars/ and verify_cv/).

All Submissions:

  • Have you followed the guidelines stated in CONTRIBUTING.md?
  • Have you checked to ensure there aren't any other open Pull Requests for the desired changed?

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (relative to 0.11.0a1 only: $ in names is rejected; one name with two defaults fails the compile)
  • This change requires a documentation update (included)

Changes To Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Defects

rx.client_state is new public API in 0.11 (#6936); this makes its contract strict before 0.11.0 ships.

  • FINDING-034: the "give it a name" hint and the ClientStateVar.create docstring showed rx.client_state("my_name"), which creates an unnamed var whose default is "my_name" (following the hint raised the same error).
  • FINDING-035: generated JS identifiers collided. name="item" and name="Item" both declared setItem (Identifier setItem has already been declared: the memo module 500s in dev, the prod build fails); one name declared with two different defaults was silently resolved by whichever mounted first, or with auto_memoize=False 500'd every route (Identifier openRxClientState has already been declared). Nothing was checked in Python; name="cs$0" was accepted and clashed with the first unnamed var.

Fix

  • Hint and docstring now show rx.client_state(default, name="my_name"). When an unnamed var's default looks like a name (the deprecated API's name-first order), the error raised by push / retrieve / global_value / global_set says so. Signature unchanged (name was already keyword-only).
  • Collision-free bindings: getter <name>RxClientState, setter <name>RxClientStateSet, updater placeholder prev<N>RxClientStateArg — injective in the name and mutually distinct (the old placeholder could also shadow a var named prev0).
  • Name rule: [A-Za-z_][A-Za-z0-9_]* (ASCII identifier without $, which is reserved for generated names); prefix uses the same validator; the error names the bad value. Every name in the docs passes. The deprecated rx._x.client_state / ClientStateVar.create apply the same rule and keep their argument order and deprecation warnings.
  • Conflicting declarations: one name with two different defaults raises ClientStateConflictError (a new ReflexError / ValueError subclass in reflex_base.utils.exceptions) at compile time, naming both declarations with file:line. The check runs as hooks are rendered (templates._render_hooks), across one App._compile (a fresh record per compile), so creating a var never raises and a dev hot reload with an edited default works. Identical re-declarations stay allowed (the documented "any component can read and write it" pattern).
  • Docs: one paragraph in docs/wrapping-react/overview.md on the name rule and the same-default rule.

Before / after:

  • item + Item: setItem declared twice → itemRxClientState, itemRxClientStateSet and ItemRxClientState, ItemRxClientStateSet.
  • rx.client_state("x", name="a-b") → name 'a-b' is not a valid client state name: it must be an ASCII identifier, a letter or underscore followed by letters, digits or underscores. It must not contain '$', which is reserved for generated names.
  • Two modules declaring name="dup" with different defaults → Client state var 'dup' is declared with two different defaults: "dup-default" (declared at …/shared.py:15) and "OTHER-default" (declared at …/other_mod.py:6). …

Test plan

  • Regression tests in tests/units/reflex_base/test_client_state.py, tests/units/experimental/test_client_state.py, tests/units/test_app.py: 17 fail on main (hint ×4 accessors, docstring, name-as-default hint, case-only identifiers, no duplicate const in the compiled memo module, placeholder shadowing, name rejection ×4, non-string message, same-body conflict, legacy name rule, app-level two-page conflict naming both sites), e.g. assert 3 == 4 … {'ItemRxClientState','itemRxClientState','setItem'}, DID NOT RAISE ValueError; allow-path tests (identical declarations, documented names, legacy positional name, conflict record not outliving a compile) pass before and after. Existing setter/placeholder spellings updated in ~20 assertions (test_client_state.py, test_memoize_plugin.py).
  • Full unit suite: 15663 passed, 66 skipped, 81.44% (the Playwright browser-driver module excluded locally for lack of a Chromium binary); touched modules rerun after the ClientStateConflictError change. ruff, pyright, pre-commit: clean.
  • End to end (dev and prod): the QA casing pages render (/casing2 used to 500), also with auto_memoize=False; hot reload with an edited default recompiles; the duplicate-default app fails at startup with the error above, the same-default variant renders all 5 routes; reflex-examples overkey (deprecated ClientStateVar.create, .set_value) 6/6 with its deprecation warning.

Notes

  • Reported, not changed: .set(value) / push(value) are not type-checked against the var's type; prefix is ignored when name is given (documented); defaults that render differently (0 vs 0.0) count as a conflict; types are not compared.

News fragments: packages/reflex-base/news/+client-state-contract.bugfix.md, news/+client-state-contract.bugfix.md.


Generated by Claude Code

View guided diff Turn on auto-fix

rx.client_state is new public API in 0.11, so make its contract strict and
its errors precise (FINDING-034, FINDING-035 of the 0.11.0a1 QA run).

- The "not global" error and the ClientStateVar.create docstring show
  rx.client_state(default, name="my_name"); the old hint passed the name
  as the default and reproduced the error. When an unnamed var's default
  is an identifier-like string (the name-first order of the deprecated
  API), the error says so and shows the keyword call.
- Generated JS bindings are the name plus a fixed suffix each: getter
  <name>RxClientState, setter <name>RxClientStateSet, updater placeholder
  prev<N>RxClientStateArg. They are injective in the name and pairwise
  disjoint, so item/Item no longer both declare setItem, and a var named
  prev0 cannot be shadowed by an updater's parameter.
- A name must be an ASCII identifier without '$' ('$' is reserved for
  generated names, so name="cs$0" can no longer alias an unnamed var);
  the error names the bad value and the rule. The deprecated
  rx._x.client_state / ClientStateVar.create apply the same rule and keep
  their argument order and warnings.
- One name declared with two different defaults fails the compile,
  naming both defaults and their file:line, instead of the first mounted
  declaration silently winning (or, with auto_memoize=False, a duplicate
  const declaration taking down every route). The check runs where hooks
  are rendered, across every body of one App._compile, so creating vars
  never raises and a hot reload with an edited default compiles. Identical
  re-declarations stay allowed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dxjadq3qkhKnqZR2b9WBFi
A name declared with two different defaults now raises
ClientStateConflictError (ReflexError, ValueError), new in
reflex_base.utils.exceptions, like the compiler's other app-level compile
errors. The message is unchanged; name-rule errors stay ValueError.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dxjadq3qkhKnqZR2b9WBFi
@masenf
masenf requested review from a team and Alek99 as code owners November 11, 2026 08:33
@masenf masenf added this to the v0.11.0 milestone Nov 11, 2026
@masenf masenf added the on deck PRs lined up to review / merge next label Nov 11, 2026
masenf pushed a commit that referenced this pull request Nov 11, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dxjadq3qkhKnqZR2b9WBFi
@greptile-apps

greptile-apps Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge, with a non-blocking memory-retention issue still open from the previous review.

Summary

Tightens the rx.client_state contract before release.

  • Client-state names now produce distinct JavaScript bindings.
  • A compile now stops when one client-state name has different defaults.
  • Unnamed client-state errors now show the right way to name a value.

Reviews (3) · Last reviewed commit: "Address review: multiline defaults, boun..." · Reviewed by Greptile

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 12 files

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread news/+client-state-contract.bugfix.md Resolved
Comment thread tests/units/test_app.py Resolved
@codspeed

codspeed Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

Merging this PR will degrade performance by 4.16%

2 warnings

⚠️ 14 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

⚠️ 1 benchmark with a significant change ran on different hardware

Its base and head results were measured on machines with a different CPU, core count or memory, so the change might not come from the code. Run base and head on the same runner type.

Learn more about Macro Runners

See affected benchmarks

❌ 1 regressed benchmark
✅ 150 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
❌ test_process_event_burst_independent_tokens[1] 2.7 ms 2.8 ms -4.16%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing fix/client-state-api-contract (2163da8) with main (2d71015)

Open in CodSpeed

@masenf

masenf commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator Author

CodSpeed is red here (test_process_event_burst_independent_tokens[1], -4.1%), but this PR doesn't cause it. CodSpeed flags that benchmark as having run on different hardware for base and head. The same benchmark shows the same 4–5% drop on every PR in this batch (#7795, #7796, #7797, #7798, #7800, #7802, #7803), whatever code each one touches. This PR only changes the rx.client_state identifier and conflict validation, which runs at definition time, not per event. CodSpeed is not a required check; leaving it for acknowledgement on CodSpeed.


Generated by Claude Code

- Match `useClientState` declarations with re.DOTALL, so a default whose
  javascript spans lines is still compared for conflicts.
- Record a declaration site only for chosen names; a generated name
  (legacy unnamed global vars) is unique and cannot conflict, and added
  an entry per construction.
- Give the app-level client state tests a clean registration context so
  the patched config is the one the compile reads.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dxjadq3qkhKnqZR2b9WBFi
masenf pushed a commit that referenced this pull request Nov 11, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dxjadq3qkhKnqZR2b9WBFi
@FarhanAliRaza
FarhanAliRaza merged commit 96de6fe into main Nov 11, 2026
167 of 168 checks passed
@FarhanAliRaza
FarhanAliRaza deleted the fix/client-state-api-contract branch November 11, 2026 13:50
masenf added a commit that referenced this pull request Nov 11, 2026
* Tighten the rx.client_state contract before 0.11.0

rx.client_state is new public API in 0.11, so make its contract strict and
its errors precise (FINDING-034, FINDING-035 of the 0.11.0a1 QA run).

- The "not global" error and the ClientStateVar.create docstring show
  rx.client_state(default, name="my_name"); the old hint passed the name
  as the default and reproduced the error. When an unnamed var's default
  is an identifier-like string (the name-first order of the deprecated
  API), the error says so and shows the keyword call.
- Generated JS bindings are the name plus a fixed suffix each: getter
  <name>RxClientState, setter <name>RxClientStateSet, updater placeholder
  prev<N>RxClientStateArg. They are injective in the name and pairwise
  disjoint, so item/Item no longer both declare setItem, and a var named
  prev0 cannot be shadowed by an updater's parameter.
- A name must be an ASCII identifier without '$' ('$' is reserved for
  generated names, so name="cs$0" can no longer alias an unnamed var);
  the error names the bad value and the rule. The deprecated
  rx._x.client_state / ClientStateVar.create apply the same rule and keep
  their argument order and warnings.
- One name declared with two different defaults fails the compile,
  naming both defaults and their file:line, instead of the first mounted
  declaration silently winning (or, with auto_memoize=False, a duplicate
  const declaration taking down every route). The check runs where hooks
  are rendered, across every body of one App._compile, so creating vars
  never raises and a hot reload with an edited default compiles. Identical
  re-declarations stay allowed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dxjadq3qkhKnqZR2b9WBFi

* Raise client state declaration conflicts as a ReflexError

A name declared with two different defaults now raises
ClientStateConflictError (ReflexError, ValueError), new in
reflex_base.utils.exceptions, like the compiler's other app-level compile
errors. The message is unchanged; name-rule errors stay ValueError.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dxjadq3qkhKnqZR2b9WBFi

* Address review: multiline defaults, bounded sites, test config isolation

- Match `useClientState` declarations with re.DOTALL, so a default whose
  javascript spans lines is still compared for conflicts.
- Record a declaration site only for chosen names; a generated name
  (legacy unnamed global vars) is unique and cannot conflict, and added
  an entry per construction.
- Give the app-level client state tests a clean registration context so
  the patched config is the one the compile reads.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dxjadq3qkhKnqZR2b9WBFi

---------

Co-authored-by: Claude <noreply@anthropic.com>
(cherry picked from commit 96de6fe)
masenf pushed a commit that referenced this pull request Nov 11, 2026
…the train

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Dxjadq3qkhKnqZR2b9WBFi
@masenf masenf added the release-train Issues found during agentic /prerelease-testing fanout, not necessarily validated bugs. label Nov 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

on deck PRs lined up to review / merge next release-train Issues found during agentic /prerelease-testing fanout, not necessarily validated bugs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants